fix(trino): pin a session schema for unqualified statements - #721

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

Thanks for the patch. One check is red and it is not your code being wrong, so here is what it is and the one-line fix.

GitGuardian Security Checks reports "1 secret uncovered".
The trigger is the only credential-shaped literal in the diff:

tests/unit/seed/types.test.ts:234 : password: "s3cret"

Its generic-password detector matches password: "" without weighing entropy. Because the PR comes from a fork, GitGuardian cannot attach the finding to an incident, so there is no detail page to read behind the check, just the summary.

Two things worth knowing:

• The check is not in the required set for main (Lint, Typecheck and Build, Unit & Integration Tests, Secret Scan), so it does not
  block a merge.
• Our own Secret Scan will not flag it. The same literal already sits in several test files and gitleaks' default rules do not report
  it, so no allowlist entry is needed.

Simplest fix: drop the password line from that case. The test asserts SeedConnectionSchema's schema field and the password never reaches an assertion. A placeholder such as "not-a-real-password" works too if you would rather keep the field populated.

Separately, the three required checks have not run yet: fork PRs wait for a maintainer to approve workflows. I will approve them so we get a real signal on the change itself.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The required checks have run now and they are green. The parts of this that were easy to get wrong, you got right: you found every reader of the connection field list including the two that fail nothing at compile time (src/lib/seed/connection-filter.ts and connectionIdentity), you marked schema as "edited" in FIELD_OWNERSHIP so clearing the field genuinely clears it rather than falling back to the stored value, and the header test asserts both arms, which the issue asked for because the negative alone would also pass a build that never sends the header at all.

My earlier request about tests/unit/seed/types.test.ts:234 still stands. While you are in there, five more, all small:

1. src/lib/db/providers/sql/trino/index.ts:203 still ends "Qualify names in full instead." That is the same copy Trino runs no unqualified statement: the connection pins a catalog and never a schema #653 asked to have re-read, and someone typing USE memory.default is exactly the person who needs to hear the connection now has a Schema field. docs/providers/trino.md:424 quotes it verbatim, so both move together.
2. docs/SEED_CONNECTIONS.md gained nothing, but you added schema to SeedConnectionSchema and to the filter, and docs/providers/trino.md:494 now claims seeded connections support it. The field table ends at connections[].authSource and the commented example does not mention it. No guard reads that file, so it has to be by hand.
3. docs/API_DOCS.md:549 is now stale: "a connection is host + port (+ optional database, username)".
4. docs/API_DOCS.md:1268-1272 lists the fields the server reads and omits schema, while the interface block twelve lines below has it. You classify it as "resolution" yourself in use-connection-payload.ts, so it belongs in that sentence.
5. src/components/ConnectionModal.tsx:558 has no icon wrapper, so its label sits flush left while "Catalog Name" above it is indented, and it uses font-medium where every neighbouring label uses font-mediumr, a long-standing typo of ours that Tailwind emits nothing for. The new label therefore renders heavier than all its siblings. Match the neighbours for now and I will fix the typo repo-wide separately.

Optional: tests/unit/db/trino/http-transport.test.ts never asserts that a connection-pinned schema alone reaches the header, because both edited cases set options.schema and shadow it. Setting this.schema = undefined leaves all 96 tests in that file green; only the integration file catches it.

Two things I am tracking rather than asking for: with the schema left empty, Create Table and the import modal's new-table path still emit unqualified statements the coordinator refuses, which #653 anticipated under Option 3; and RESOLVABLE_FIELDS does not cover schema, the same as every other engine-specific field.

Thanks for the detailed review. I’ve addressed all requested changes: removed the test password that triggered GitGuardian, updated the Trino session warning and matching documentation, documented the schema field in Seed Connections and the API reference, and aligned the Schema label layout and styling with its neighbors. I also added a direct unit test for the connection-pinned schema header and strengthened the warning assertion. I left the two tracked follow-ups outside this PR as noted. The full test suite, build, and coverage checks pass with 100% line coverage.

All six are addressed and I checked each one rather than taking the summary. The three required checks are green.

The optional test you added is the one I care about most, and it is real: setting this.schema = undefined in the constructor turns exactly that test red and leaves the other 96 in the file passing. Same for the warning copy, reverting the sentence in index.ts fails the integration assertion, and both prose copies of it moved together, so the doc cannot drift from the string.

One thing not to chase: GitGuardian will stay red on this branch. It scans commits, not the working tree, and the literal is still in your first commit. The line is gone from seed/types.test.ts, our own Secret Scan is green, the check is not required, and we squash-merge, so nothing reaches main. No history rewrite needed.

One cosmetic leftover I will take myself after merge: in docs/SEED_CONNECTIONS.md the new schema: comment column sits one space right of database: above it. Not worth a push.

Good work on this one. Thanks for the care on the field-list readers and on the test that asserts both arms. I will merge it shortly.